🎖️GitЯра🎖️
Commit a2763bdfebb0885a229057f7a812d8b5775b7400
Parents : 72b981f
Author : James Rich <2199651+jamesarich@users.noreply.github.com>
Signature : Signature validation error
Date : 2026-04-14T21:20:33-05:00
Committer : GitHub <noreply@github.com>
Date : 2026-04-15T02:20:33Z
fix(charts): apply Vico 3.1.0 best-practice audit fixes (#5138)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Changes
6 files changed, 110 insertions(+), 62 deletions(-)
Diff
diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/BaseMetricChart.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/BaseMetricChart.kt
index 8f65bf6d88..a425e272dc 100644
--- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/BaseMetricChart.kt
+++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/BaseMetricChart.kt
@@ -35,6 +35,7 @@ import androidx.compose.material3.IconToggleButton
import androidx.compose.material3.Scaffold
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
+import androidx.compose.runtime.key
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
@@ -79,6 +80,9 @@ import org.meshtastic.core.ui.icon.MeshtasticIcons
import org.meshtastic.core.ui.icon.Refresh
import org.meshtastic.core.ui.icon.Save
+/** Minimum x-step (in seconds) to prevent the default GCD from producing a value of 1 with irregular timestamps. */
+private const val MIN_X_STEP_SECONDS = 60.0
+
/**
* A generic chart host for Meshtastic metric charts. Handles common boilerplate for markers, scrolling, and point
* selection synchronization.
@@ -100,43 +104,50 @@ fun GenericMetricChart(
onPointSelected: ((Double) -> Unit)? = null,
vicoScrollState: VicoScrollState = rememberVicoScrollState(),
) {
- // Hoist zoom state above rememberCartesianChart so that the variable slot count
- // from the vararg layers spread does not shift this remember call during recomposition
- // (toggling legend chips changes the layer count, which corrupts the slot table).
- val zoomState = rememberVicoZoomState(zoomEnabled = true, initialZoom = Zoom.Content)
+ // Key on layer count so Compose rebuilds the entire subtree when legend chip toggles
+ // add/remove layers. rememberCartesianChart uses vararg internally, so changing the
+ // argument count without a key corrupts the slot table.
+ key(layers.size) {
+ val zoomState = rememberVicoZoomState(zoomEnabled = true, initialZoom = Zoom.Content)
- val markerVisibilityListener =
- remember(onPointSelected) {
- object : CartesianMarkerVisibilityListener {
- override fun onShown(marker: CartesianMarker, targets: List<CartesianMarker.Target>) {
- targets.firstOrNull()?.let { onPointSelected?.invoke(it.x) }
- }
+ val markerVisibilityListener =
+ remember(onPointSelected) {
+ object : CartesianMarkerVisibilityListener {
+ override fun onShown(marker: CartesianMarker, targets: List<CartesianMarker.Target>) {
+ targets.firstOrNull()?.let { onPointSelected?.invoke(it.x) }
+ }
- override fun onUpdated(marker: CartesianMarker, targets: List<CartesianMarker.Target>) {
- targets.firstOrNull()?.let { onPointSelected?.invoke(it.x) }
+ override fun onUpdated(marker: CartesianMarker, targets: List<CartesianMarker.Target>) {
+ targets.firstOrNull()?.let { onPointSelected?.invoke(it.x) }
+ }
}
}
- }
- CartesianChartHost(
- chart =
- @Suppress("SpreadOperator")
- rememberCartesianChart(
- *layers.toTypedArray(),
- startAxis = startAxis,
- endAxis = endAxis,
- bottomAxis = bottomAxis,
- marker = marker,
- markerVisibilityListener = markerVisibilityListener,
- persistentMarkers = { _ -> if (selectedX != null && marker != null) marker at selectedX else null },
- fadingEdges = rememberFadingEdges(),
- decorations = decorations,
- ),
- modelProducer = modelProducer,
- modifier = modifier,
- scrollState = vicoScrollState,
- zoomState = zoomState,
- )
+ CartesianChartHost(
+ chart =
+ @Suppress("SpreadOperator")
+ rememberCartesianChart(
+ *layers.toTypedArray(),
+ startAxis = startAxis,
+ endAxis = endAxis,
+ bottomAxis = bottomAxis,
+ marker = marker,
+ markerVisibilityListener = markerVisibilityListener,
+ persistentMarkers = { _ -> if (selectedX != null && marker != null) marker at selectedX else null },
+ fadingEdges = rememberFadingEdges(),
+ decorations = decorations,
+ // Telemetry timestamps arrive at irregular intervals. Without an explicit
+ // x-step, Vico computes the GCD of consecutive x-value differences which can
+ // be as small as 1 second, making the chart logically enormous. A 60-second
+ // floor keeps the internal slot count reasonable for any practical interval.
+ getXStep = { model -> maxOf(model.getXDeltaGcd(), MIN_X_STEP_SECONDS) },
+ ),
+ modelProducer = modelProducer,
+ modifier = modifier,
+ scrollState = vicoScrollState,
+ zoomState = zoomState,
+ )
+ }
}
/**
diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/ChartStyling.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/ChartStyling.kt
index c1cf0e04e5..da8b16e47c 100644
--- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/ChartStyling.kt
+++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/ChartStyling.kt
@@ -57,7 +57,7 @@ import com.patrykandpatrick.vico.compose.common.component.rememberTextComponent
* **Design principles** (per [design#53](https://github.com/meshtastic/design/issues/53)):
* - Default to thin lines **without** point markers to avoid clutter on dense timeseries.
* - Show a single dot only at the marker/cursor position (handled by [rememberMarker]).
- * - Use `Interpolator.catmullRom()` for smooth curves that pass through every data point.
+ * - Use `Interpolator.cubic()` for smooth monotone curves that won't overshoot between sparse points.
* - Reserve bold lines for the single most-important series; use subtle/gradient fills for secondary data.
*/
@Suppress("TooManyFunctions")
@@ -73,15 +73,21 @@ object ChartStyling {
*
* @param lineColor The color of the line
* @param lineWidth Width of the line in dp
+ * @param interpolator The line interpolation strategy. Defaults to monotone
+ * [cubic][LineCartesianLayer.Interpolator.cubic] which won't overshoot between sparse data points (unlike
+ * catmull-rom). Use [Sharp][LineCartesianLayer.Interpolator.Sharp] for discrete/integer metrics like hop counts.
* @return Configured [LineCartesianLayer.Line]
*/
@Composable
- fun createStyledLine(lineColor: Color, lineWidth: Float = MEDIUM_LINE_WIDTH_DP): LineCartesianLayer.Line =
- LineCartesianLayer.rememberLine(
- fill = LineCartesianLayer.LineFill.single(Fill(lineColor)),
- stroke = LineCartesianLayer.LineStroke.Continuous(lineWidth.dp),
- interpolator = LineCartesianLayer.Interpolator.catmullRom(),
- )
+ fun createStyledLine(
+ lineColor: Color,
+ lineWidth: Float = MEDIUM_LINE_WIDTH_DP,
+ interpolator: LineCartesianLayer.Interpolator = LineCartesianLayer.Interpolator.cubic(),
+ ): LineCartesianLayer.Line = LineCartesianLayer.rememberLine(
+ fill = LineCartesianLayer.LineFill.single(Fill(lineColor)),
+ stroke = LineCartesianLayer.LineStroke.Continuous(lineWidth.dp),
+ interpolator = interpolator,
+ )
/**
* Creates a line with a gradient area fill effect. Ideal for emphasising a single series or showing magnitude. The
@@ -92,14 +98,18 @@ object ChartStyling {
* @return Configured [LineCartesianLayer.Line]
*/
@Composable
- fun createGradientLine(lineColor: Color, lineWidth: Float = MEDIUM_LINE_WIDTH_DP): LineCartesianLayer.Line {
+ fun createGradientLine(
+ lineColor: Color,
+ lineWidth: Float = MEDIUM_LINE_WIDTH_DP,
+ interpolator: LineCartesianLayer.Interpolator = LineCartesianLayer.Interpolator.cubic(),
+ ): LineCartesianLayer.Line {
val gradientBrush =
Brush.verticalGradient(colors = listOf(lineColor.copy(alpha = 0.3f), lineColor.copy(alpha = 0.05f)))
return LineCartesianLayer.rememberLine(
fill = LineCartesianLayer.LineFill.single(Fill(lineColor)),
areaFill = LineCartesianLayer.AreaFill.single(Fill(gradientBrush)),
stroke = LineCartesianLayer.LineStroke.Continuous(lineWidth.dp),
- interpolator = LineCartesianLayer.Interpolator.catmullRom(),
+ interpolator = interpolator,
)
}
@@ -110,8 +120,11 @@ object ChartStyling {
* @return Configured [LineCartesianLayer.Line]
*/
@Composable
- fun createBoldLine(lineColor: Color): LineCartesianLayer.Line =
- createStyledLine(lineColor = lineColor, lineWidth = THICK_LINE_WIDTH_DP)
+ fun createBoldLine(
+ lineColor: Color,
+ interpolator: LineCartesianLayer.Interpolator = LineCartesianLayer.Interpolator.cubic(),
+ ): LineCartesianLayer.Line =
+ createStyledLine(lineColor = lineColor, lineWidth = THICK_LINE_WIDTH_DP, interpolator = interpolator)
/**
* Creates a subtle line suitable for secondary metrics that should not dominate the chart.
@@ -131,7 +144,10 @@ object ChartStyling {
* @return Configured [LineCartesianLayer.Line]
*/
@Composable
- fun createDashedLine(lineColor: Color): LineCartesianLayer.Line = LineCartesianLayer.rememberLine(
+ fun createDashedLine(
+ lineColor: Color,
+ interpolator: LineCartesianLayer.Interpolator = LineCartesianLayer.Interpolator.cubic(),
+ ): LineCartesianLayer.Line = LineCartesianLayer.rememberLine(
fill = LineCartesianLayer.LineFill.single(Fill(lineColor)),
stroke =
LineCartesianLayer.LineStroke.Dashed(
@@ -139,7 +155,7 @@ object ChartStyling {
dashLength = 6.dp,
gapLength = 3.dp,
),
- interpolator = LineCartesianLayer.Interpolator.catmullRom(),
+ interpolator = interpolator,
)
/**
diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/DeviceMetrics.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/DeviceMetrics.kt
index 1e749d22e1..609048a926 100644
--- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/DeviceMetrics.kt
+++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/DeviceMetrics.kt
@@ -307,12 +307,13 @@ private fun DeviceMetricsChart(
}
}
+ val percentRangeProvider = remember { CartesianLayerRangeProvider.fixed(minY = 0.0, maxY = 100.0) }
val leftLayer =
rememberConditionalLayer(
hasData = leftLayerSeriesStyles.isNotEmpty(),
lineProvider = LineCartesianLayer.LineProvider.series(leftLayerSeriesStyles),
verticalAxisPosition = Axis.Position.Vertical.Start,
- rangeProvider = CartesianLayerRangeProvider.fixed(minY = 0.0, maxY = 100.0),
+ rangeProvider = percentRangeProvider,
)
val rightLayer =
diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/EnvironmentCharts.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/EnvironmentCharts.kt
index 0f809ef818..5029729ca0 100644
--- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/EnvironmentCharts.kt
+++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/EnvironmentCharts.kt
@@ -158,11 +158,11 @@ fun EnvironmentMetricsChart(
graphData.shouldPlot[(it.metricKey as? Environment)?.ordinal ?: 0]
}
- // Legend toggle state: tracks indices into allLegendData that are hidden
- var hiddenIndices by remember { mutableStateOf(emptySet<Int>()) }
- val hiddenMetrics =
- remember(hiddenIndices, allLegendData) {
- hiddenIndices.mapNotNull { allLegendData.getOrNull(it)?.metricKey as? Environment }.toSet()
+ // Track hidden metrics by key (not index) so toggling survives changes in allLegendData ordering.
+ var hiddenMetrics by remember { mutableStateOf(emptySet<Environment>()) }
+ val hiddenIndices =
+ remember(hiddenMetrics, allLegendData) {
+ allLegendData.indices.filter { (allLegendData[it].metricKey as? Environment) in hiddenMetrics }.toSet()
}
val colorToLabel = allLegendData.associate { it.color to (it.labelOverride ?: stringResource(it.nameRes)) }
@@ -233,6 +233,7 @@ fun EnvironmentMetricsChart(
},
)
+ val pressureRangeProvider = remember { CartesianLayerRangeProvider.fixed(minY = 700.0, maxY = 1200.0) }
val layers = mutableListOf<LineCartesianLayer>()
if (showPressure && pressureData.isNotEmpty()) {
layers.add(
@@ -244,7 +245,7 @@ fun EnvironmentMetricsChart(
verticalAxisPosition = Axis.Position.Vertical.Start,
// Fixed range per Oscar's UX guidance: barometric pressure should NOT autoscale,
// otherwise trends (storms) are invisible. 700-1200 hPa covers sea-level to altitude.
- rangeProvider = CartesianLayerRangeProvider.fixed(minY = 700.0, maxY = 1200.0),
+ rangeProvider = pressureRangeProvider,
),
)
}
@@ -254,7 +255,7 @@ fun EnvironmentMetricsChart(
when (metric) {
Environment.RADIATION,
Environment.WIND_SPEED,
- -> CartesianLayerRangeProvider.fixed(minY = 0.0)
+ -> CartesianLayerRangeProvider.auto()
else -> null
}
val lineStyle =
@@ -310,7 +311,8 @@ fun EnvironmentMetricsChart(
modifier = Modifier.padding(top = 0.dp),
hiddenSet = hiddenIndices,
onToggle = { index ->
- hiddenIndices = if (index in hiddenIndices) hiddenIndices - index else hiddenIndices + index
+ val metric = allLegendData.getOrNull(index)?.metricKey as? Environment ?: return@Legend
+ hiddenMetrics = if (metric in hiddenMetrics) hiddenMetrics - metric else hiddenMetrics + metric
},
)
}
diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/PaxMetrics.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/PaxMetrics.kt
index 598cd5ca9a..b3b0b36e05 100644
--- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/PaxMetrics.kt
+++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/PaxMetrics.kt
@@ -141,11 +141,20 @@ private fun PaxMetricsChart(
rememberLineCartesianLayer(
lineProvider =
LineCartesianLayer.LineProvider.series(
- ChartStyling.createGradientLine(lineColor = bleColor),
- ChartStyling.createGradientLine(lineColor = wifiColor),
- ChartStyling.createBoldLine(lineColor = paxColor),
+ ChartStyling.createGradientLine(
+ lineColor = bleColor,
+ interpolator = LineCartesianLayer.Interpolator.Sharp,
+ ),
+ ChartStyling.createGradientLine(
+ lineColor = wifiColor,
+ interpolator = LineCartesianLayer.Interpolator.Sharp,
+ ),
+ ChartStyling.createBoldLine(
+ lineColor = paxColor,
+ interpolator = LineCartesianLayer.Interpolator.Sharp,
+ ),
),
- rangeProvider = CartesianLayerRangeProvider.fixed(minY = 0.0),
+ rangeProvider = CartesianLayerRangeProvider.auto(),
),
),
startAxis = VerticalAxis.rememberStart(label = axisLabel),
diff --git a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteChart.kt b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteChart.kt
index c1e5e69fe7..c27f111d14 100644
--- a/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteChart.kt
+++ b/feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/metrics/TracerouteChart.kt
@@ -189,17 +189,26 @@ internal fun TracerouteMetricsChart(
val forwardLayer =
rememberConditionalLayer(
hasData = forwardData.isNotEmpty(),
- lineProvider = LineCartesianLayer.LineProvider.series(ChartStyling.createStyledLine(forwardColor)),
+ lineProvider =
+ LineCartesianLayer.LineProvider.series(
+ ChartStyling.createStyledLine(
+ forwardColor,
+ interpolator = LineCartesianLayer.Interpolator.Sharp,
+ ),
+ ),
verticalAxisPosition = Axis.Position.Vertical.Start,
- rangeProvider = CartesianLayerRangeProvider.fixed(minY = 0.0),
+ rangeProvider = CartesianLayerRangeProvider.auto(),
)
val returnLayer =
rememberConditionalLayer(
hasData = returnData.isNotEmpty(),
- lineProvider = LineCartesianLayer.LineProvider.series(ChartStyling.createDashedLine(returnColor)),
+ lineProvider =
+ LineCartesianLayer.LineProvider.series(
+ ChartStyling.createDashedLine(returnColor, interpolator = LineCartesianLayer.Interpolator.Sharp),
+ ),
verticalAxisPosition = Axis.Position.Vertical.Start,
- rangeProvider = CartesianLayerRangeProvider.fixed(minY = 0.0),
+ rangeProvider = CartesianLayerRangeProvider.auto(),
)
val rttLayer =
Served by rngit 1.5.0 - Generated in 0.13s